[ios]enable the webview non tappable workaround by checking subviews recursively by hellohuanlin · Pull Request #57168 · flutter/engine

hellohuanlin · 2024-12-12T21:45:15Z

The original workaround (PR) works for the official web view plugin, but it doesn't work for a third party plugin flutter_inappwebview (issue). Upon discussion with the author of that plugin, it turns out that their platform view is not a WKWebView, but rather a wrapper of WKWebView.

This PR performs a DFS search of the view hierarchy, and enable the workaround as long as there's a WKWebView inside.

TODO: pending sample project:
I am quite positive that it should work, but I haven't tried it since I don't have a sample project yet. I have requested a sample project with them so I can verify the solution.

List which issues are fixed by this PR. You must list at least one issue.

pichillilorenzo/flutter_inappwebview#2415

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

jmagman · 2024-12-12T22:22:09Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

          [self removeGestureRecognizer:self.delayingRecognizer];
          [self addGestureRecognizer:self.delayingRecognizer];


Will this cause problems in the views they are gesturing into that are not WKWebViews?

I think it is possible. That's something that we need to watch out for. But I highly doubt if it is gonna cause real issues

We are talking about an odd of combining:

having a setup with webview + another non-web-view subview that also handles gesture

having a flutter overlay layer on top of this non-web-view subview

User touch the area inside non-web-view subview, but outside of the overlay layer

Then we will have this non-web-view subview receive the extra touch when it's not supposed to.

Then even when this happens, I don't think it is a huge problem.

hellohuanlin · 2024-12-12T23:28:52Z

I was able to verify this solution works in the sample project provided by the author of flutter_inappwebview

jmagman

RSLGTM

…ubviews recursively (flutter/engine#57168)

flutter/engine@9b51e30...5eedfef 2024-12-13 flar@google.com Normalize round rect bounds when coming from Flutter (flutter/engine#57171) 2024-12-13 41930132+hellohuanlin@users.noreply.github.com [ios]enable the webview non tappable workaround by checking subviews recursively (flutter/engine#57168) 2024-12-12 30870216+gaaclarke@users.noreply.github.com removed c style casts and enabled the lint (flutter/engine#57162) 2024-12-12 jonahwilliams@google.com [Impeller] exploit perfect hash for SamplerDescriptor. (flutter/engine#57036) 2024-12-12 30870216+gaaclarke@users.noreply.github.com Reenabled labelling test with a capabilities check. (flutter/engine#57160) 2024-12-12 jonahwilliams@google.com [Impeller] dont print format strings for blend filter and snapshots. (flutter/engine#57105) 2024-12-12 robert.ancell@canonical.com Make fl_engine_send_key_event into a standard async function. (flutter/engine#57112) 2024-12-12 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from HJ57Y3zxqDamI8qkY... to iWMEbVYaNdH8RJmXZ... (flutter/engine#57163) 2024-12-12 robert.ancell@canonical.com Migrate FlPlatformChannel tests to FlMockBinaryMessenger (flutter/engine#57140) 2024-12-12 robert.ancell@canonical.com Migrate FlBasicMessageChannel tests to FlMockBinaryMessenger (flutter/engine#57115) 2024-12-12 flar@google.com Migrate layers and layer_tree to DisplayList/Impeller geometry classes (flutter/engine#57153) 2024-12-12 mdebbar@google.com [web] Use CanvasKit to run tests under engine/ (flutter/engine#54786) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from HJ57Y3zxqDam to iWMEbVYaNdH8 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jonahwilliams@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This PR limits the search depth, because we don't want to enable this workaround for AdMob banner, which has a WKWebView in the depth of 7. See the previous PR for more context: #57168 I was able to confirm that this returns YES for the 3P plugin, and NO for AdMob. *List which issues are fixed by this PR. You must list at least one issue.* flutter/flutter#158961 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

… subviews recursively #57168 (#57172) CP for #57168 and #57193 *List which issues are fixed by this PR. You must list at least one issue.* flutter/flutter#158961 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

… AND "enable the webview non tappable workaround by checking subviews recursively #57168" (#57176) CP for 3 PRs: #56804 and #57168 and #57193 This is for 3.28. Since the previous PR was not merged, so I combined the 2 PRs to make it easier to merge. *List which issues are fixed by this PR. You must list at least one issue.* pichillilorenzo/flutter_inappwebview#2415 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

…recursively (flutter/engine#57168) The original workaround ([PR](flutter/engine#56804)) works for the official web view plugin, but it doesn't work for a third party plugin `flutter_inappwebview` ([issue](https://github.com/pichillilorenzo/flutter_inappwebview)). Upon discussion with the author of that plugin, it turns out that their platform view is not a WKWebView, but rather a wrapper of WKWebView. This PR performs a DFS search of the view hierarchy, and enable the workaround as long as there's a WKWebView inside. TODO: pending sample project: I am quite positive that it should work, but **I haven't tried it since I don't have a sample project yet**. I have requested a sample project with them so I can verify the solution. *List which issues are fixed by this PR. You must list at least one issue.* pichillilorenzo/flutter_inappwebview#2415 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

…er/engine#57193) This PR limits the search depth, because we don't want to enable this workaround for AdMob banner, which has a WKWebView in the depth of 7. See the previous PR for more context: flutter/engine#57168 I was able to confirm that this returns YES for the 3P plugin, and NO for AdMob. *List which issues are fixed by this PR. You must list at least one issue.* flutter#158961 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

… subviews recursively flutter#57168 (flutter#57172) CP for flutter#57168 and flutter#57193 *List which issues are fixed by this PR. You must list at least one issue.* flutter/flutter#158961 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

[ios]enable the webview non tappable workaround by checking views

1b5fc9e

github-actions bot added the platform-ios label Dec 12, 2024

hellohuanlin requested review from cbracken and jmagman December 12, 2024 21:45

hellohuanlin changed the title ~~[ios]enable the webview non tappable workaround by checking views~~ [ios]enable the webview non tappable workaround by checking subviews recursively Dec 12, 2024

hellohuanlin mentioned this pull request Dec 12, 2024

Tap interactions not working on iOS 18.2 beta for webview_flutter 4.10.0 flutter/flutter#158961

Closed

jmagman reviewed Dec 12, 2024

View reviewed changes

jmagman approved these changes Dec 13, 2024

View reviewed changes

hellohuanlin mentioned this pull request Dec 13, 2024

[CP][3.27][ios]enable the webview non tappable workaround by checking subviews recursively #57168 #57172

Merged

8 tasks

hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 13, 2024

hellohuanlin mentioned this pull request Dec 13, 2024

[cp][3.28] Combined CP for "workaround for non-tappable webview #56804" AND "enable the webview non tappable workaround by checking subviews recursively #57168" #57176

Merged

8 tasks

auto-submit bot merged commit 4402232 into flutter:main Dec 13, 2024
28 checks passed

engine-flutter-autoroll mentioned this pull request Dec 13, 2024

Roll Flutter Engine from 9b51e30a4d37 to 5eedfefe38c1 (12 revisions) flutter/flutter#160220

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 13, 2024

4402232 [ios]enable the webview non tappable workaround by checking s…

9c226ac

…ubviews recursively (flutter/engine#57168)

hellohuanlin mentioned this pull request Dec 13, 2024

[ios]limit web view not tappable workaround to a limited depth #57193

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios]enable the webview non tappable workaround by checking subviews recursively#57168

[ios]enable the webview non tappable workaround by checking subviews recursively#57168
auto-submit[bot] merged 1 commit intoflutter:mainfrom
hellohuanlin:pv_webview_not_tappable_dfs_subviews

hellohuanlin commented Dec 12, 2024 •

edited by jmagman

Loading

Uh oh!

jmagman Dec 12, 2024

Uh oh!

hellohuanlin Dec 12, 2024

Uh oh!

hellohuanlin Dec 12, 2024 •

edited

Loading

Uh oh!

hellohuanlin commented Dec 12, 2024

Uh oh!

jmagman left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		[self removeGestureRecognizer:self.delayingRecognizer];
		[self addGestureRecognizer:self.delayingRecognizer];

Conversation

hellohuanlin commented Dec 12, 2024 • edited by jmagman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

jmagman Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Dec 12, 2024

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin commented Dec 12, 2024

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hellohuanlin commented Dec 12, 2024 •

edited by jmagman

Loading

hellohuanlin Dec 12, 2024 •

edited

Loading